Make High-Level-Rest-Client tests allow deprecation warning temporarily, during deprecation of request parameter 'master_timeout' by tlfeng · Pull Request #2702 · opensearch-project/OpenSearch

tlfeng · 2022-04-01T08:41:15Z

Description

Change:

Make Rest-High-Rest-Level tests allow deprecation warning temporarily, by changing the argument of setStrictDeprecationMode() to false when building RestClient for tests.

Reason:
Many High-Level-Rest-Client tests are broken due to deprecation warning, during the process of deprecating master_timeout parameter of REST APIs.
The simplest solution is:

Set "StrictDeprecationMode" to false to make tests allow deprecation warning.
Add alternative parameter cluster_manager_timeout for all applicable REST APIs
Change to use cluster_manager_timeout instead of master_timeout in High-Level-Rest-Client RequestConverter class
Set "StrictDeprecationMode" back to true

Note:

The javadoc for WarningFailureException provides the way to allow warnings in the RestClient tests.
The tests will be fixed by PR Replace parameter 'master_timeout' with 'cluster_manager_tiemout' in RequestConverters of High-Level-Rest-Client #2683, after all the other PRs in issue Add a new request parameter "cluster_manager_timeout" in the REST APIs that accept "master_timeout" parameter, and deprecate "master_timeout" #2511 is merged.
Those tests are causing failures in PR Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Cluster APIs #2658 , Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Index APIs except index template APIs #2660 , Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Index Template APIs #2678 , Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Snapshot APIs #2680 and Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Ingest APIs and Script APIs #2682

A Sample test failure

> Task :client:rest-high-level:asyncIntegTest

REPRODUCE WITH: ./gradlew ':client:rest-high-level:asyncIntegTest' --tests "org.opensearch.client.ClusterClientIT.testClusterGetSettings" -Dtests.seed=F5E30873F5E8B3F6 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1 -XX:ReservedCodeCacheSize=64m" -Dtests.locale=sr-Latn-BA -Dtests.timezone=America/Santa_Isabel -Druntime.java=17

org.opensearch.client.ClusterClientIT > testClusterGetSettings FAILED
    org.opensearch.client.WarningFailureException: method [GET], host [http://127.0.0.1:38451], URI [/_cluster/settings?master_timeout=30s], status line [HTTP/1.1 200 OK]
    Warnings: [Deprecated parameter [master_timeout] used. To promote inclusive language, please use [cluster_manager_timeout] instead. It will be unsupported in a future major version.]
    {"persistent":{"cluster":{"routing":{"allocation":{"enable":"NONE"}}}},"transient":{"indices":{"recovery":{"max_bytes_per_sec":"10b"}}}}
        at __randomizedtesting.SeedInfo.seed([F5E30873F5E8B3F6:8CF8FB5CACB76F2B]:0)
        at app//org.opensearch.client.RestClient.convertResponse(RestClient.java:346)

Issues Resolved

A part of issue #2511

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Tianli Feng <ftianli@amazon.com>

opensearch-ci-bot · 2022-04-01T09:27:56Z

✅ Gradle Check success 3bda2c9
Log 4026

Reports 4026

dreamer-89 · 2022-04-01T23:22:44Z

...st-high-level/src/test/java/org/opensearch/client/OpenSearchRestHighLevelClientTestCase.java

 import static org.hamcrest.Matchers.lessThan;
 import static org.hamcrest.Matchers.notNullValue;

+@LuceneTestCase.AwaitsFix(bugUrl = "https://github.com/opensearch-project/OpenSearch/pull/2683")


Thank you for raising this PR.

I have one concern that muting test classes has potential of introducing bugs from other code changes exercising these two test classes. How do we have plan to cover this ?

Hi @dreamer-89 thank you for reviewing my PR.
My thought is to hold merging this PR until all the other PRs are ready to be merged in #2511. When getting ready, then merging all those PRs together in a short time. The potential of other code changes merged can be reduced by try to shorten the interval of muting the tests.
This is a good point, I should put up some notice saying not to merge this PR in a hurry. 😄

Thanks @tlfeng for clarifying.

Is it possible we can add OpenSearchRestTestCase.expectWarnings in tests in above two tests files wherever we are adding master_timeout. I think this is how we are tackling other deprecated features on client side.

Thank you for your good suggestion!
You are right. 👍 Looks like I should turn off the enableWarningsCheck https://github.com/opensearch-project/OpenSearch/blob/1.3.1/test/framework/src/main/java/org/opensearch/test/OpenSearchTestCase.java#L407 for the affected test classes.
There are 24 classes extends this class OpenSearchRestHighLevelClientTestCase (and one class has turned it off https://github.com/opensearch-project/OpenSearch/blob/1.3.1/client/rest-high-level/src/test/java/org/opensearch/client/BulkProcessorIT.java#L89) so the amount of work is reasonable.
I will apply it to the affected test classes.

Updated the code. The enableWarningsCheck doesn't work for this case, but instead, I found using setStrictDeprecationMode(false) to build the RestClient can make the test allow deprecation warnings. This will be a safer way 😄.

…cation warning in HLRC tests Signed-off-by: Tianli Feng <ftianli@amazon.com>

opensearch-ci-bot · 2022-04-02T02:22:33Z

✅ Gradle Check success 673e55b
Log 4064

Reports 4064

nknize

LGTM...hopefully this won't stay in flight too long.

…, during deprecation of request parameter 'master_timeout' (#2702) Temporarily build rest client with setStrictDeprecationMode(false) to allow deprecation warning in HLRC tests while master_timeout parameters is being refactored. Signed-off-by: Tianli Feng <ftianli@amazon.com> (cherry picked from commit 6a2a33d)

tlfeng · 2022-04-04T16:00:01Z

LGTM...hopefully this won't stay in flight too long.

@nknize Thanks for reviewing my PR! 👍 I will try to merging other PRs in #2511 soon and revert this change in this week.

…, during deprecation of request parameter 'master_timeout' (#2702) (#2740) Temporarily build rest client with setStrictDeprecationMode(false) to allow deprecation warning in HLRC tests while master_timeout parameters is being refactored. Signed-off-by: Tianli Feng <ftianli@amazon.com> (cherry picked from commit 6a2a33d)

…, during deprecation of request parameter 'master_timeout' (#2702) (#2741) Temporarily build rest client with setStrictDeprecationMode(false) to allow deprecation warning in HLRC tests while master_timeout parameters is being refactored. Signed-off-by: Tianli Feng <ftianli@amazon.com> (cherry picked from commit 6a2a33d)

…emporarily, during deprecation of request parameter 'master_timeout' (opensearch-project#2702)" This reverts commit 6a2a33d. Signed-off-by: Tianli Feng <ftianli@amazon.com>

…emporarily, during deprecation of request parameter 'master_timeout' (#2702)" (#2930) This reverts commit 6a2a33d. * Sets the High-Level-Rest-Client tests back to treating warning header as a failure. Signed-off-by: Tianli Feng <ftianli@amazon.com>

…emporarily, during deprecation of request parameter 'master_timeout' (#2702)" (#2930) This reverts commit 6a2a33d. * Sets the High-Level-Rest-Client tests back to treating warning header as a failure. Signed-off-by: Tianli Feng <ftianli@amazon.com> (cherry picked from commit 91aa312)

…emporarily, during deprecation of request parameter 'master_timeout' (#2702)" (#2930) (#2934) This reverts commit 6a2a33d. * Sets the High-Level-Rest-Client tests back to treating warning header as a failure. Signed-off-by: Tianli Feng <ftianli@amazon.com> (cherry picked from commit 91aa312)

…emporarily, during deprecation of request parameter 'master_timeout' (#2702)" (#2744) This reverts commit 6a2a33d. During the process of deprecating REST API request parameter master_timeout and adding alternative parameter cluster_manager_timeout, I made High-Level-Rest-Client tests allow deprecation warning temporarily, by changing the argument of `setStrictDeprecationMode()` to false when building `RestClient` for tests, in the above commit / PR #2702, This PR sets the High-Level-Rest-Client tests back to treating warning header as a failure. Signed-off-by: Tianli Feng <ftianli@amazon.com>

Mute HLRC tests that using master_timeout parameter

3bda2c9

Signed-off-by: Tianli Feng <ftianli@amazon.com>

tlfeng added :test Adding or fixing a test v2.0.0 Version 2.0.0 backport 2.x Backport to 2.x branch backport 2.0 Backport to 2.0 branch labels Apr 1, 2022

tlfeng mentioned this pull request Apr 1, 2022

Add a new request parameter "cluster_manager_timeout" in the REST APIs that accept "master_timeout" parameter, and deprecate "master_timeout" #2511

Closed

11 tasks

tlfeng changed the title ~~Mute High-Level-Rest-Client tests that using 'master_timeout' parameter temporarily~~ Mute High-Level-Rest-Client tests that REST API requests with 'master_timeout' parameter will be sent temporarily Apr 1, 2022

tlfeng changed the title ~~Mute High-Level-Rest-Client tests that REST API requests with 'master_timeout' parameter will be sent temporarily~~ Mute High-Level-Rest-Client tests where REST API requests with 'master_timeout' parameter will be sent temporarily Apr 1, 2022

tlfeng marked this pull request as ready for review April 1, 2022 09:12

tlfeng requested a review from a team as a code owner April 1, 2022 09:12

dreamer-89 reviewed Apr 1, 2022

View reviewed changes

tlfeng added the WIP Work in progress label Apr 1, 2022

This was referenced Apr 2, 2022

Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Cluster APIs #2658

Merged

Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Index APIs except index template APIs #2660

Merged

Build rest client with setStrictDeprecationMode(false) to allow depre…

673e55b

…cation warning in HLRC tests Signed-off-by: Tianli Feng <ftianli@amazon.com>

tlfeng force-pushed the mute-hlrc-test-timeout branch from a88317b to 673e55b Compare April 2, 2022 01:44

tlfeng changed the title ~~Mute High-Level-Rest-Client tests where REST API requests with 'master_timeout' parameter will be sent temporarily~~ Make Rest-High-Rest-Level tests allow deprecation warning temporarily Apr 2, 2022

tlfeng changed the title ~~Make Rest-High-Rest-Level tests allow deprecation warning temporarily~~ Make Rest-High-Rest-Level tests allow deprecation warning temporarily during deprecation of request parameter 'master_timeout' Apr 2, 2022

tlfeng changed the title ~~Make Rest-High-Rest-Level tests allow deprecation warning temporarily during deprecation of request parameter 'master_timeout'~~ Make Rest-High-Rest-Level tests allow deprecation warning temporarily, during deprecation of request parameter 'master_timeout' Apr 2, 2022

tlfeng mentioned this pull request Apr 2, 2022

Add request parameter 'cluster_manager_timeout' and deprecate 'master_timeout' - in Ingest APIs and Script APIs #2682

Merged

5 tasks

nknize approved these changes Apr 4, 2022

View reviewed changes

nknize merged commit 6a2a33d into opensearch-project:main Apr 4, 2022

opensearch-trigger-bot bot mentioned this pull request Apr 4, 2022

[Backport 2.x] Make High-Level-Rest-Client tests allow deprecation warning temporarily, during deprecation of request parameter 'master_timeout' #2740

Merged

opensearch-trigger-bot bot mentioned this pull request Apr 4, 2022

[Backport 2.0] Make High-Level-Rest-Client tests allow deprecation warning temporarily, during deprecation of request parameter 'master_timeout' #2741

Merged

tlfeng changed the title ~~Make Rest-High-Rest-Level tests allow deprecation warning temporarily, during deprecation of request parameter 'master_timeout'~~ Make High-Level-Rest-Client tests allow deprecation warning temporarily, during deprecation of request parameter 'master_timeout' Apr 4, 2022

tlfeng mentioned this pull request Apr 4, 2022

Revert "Make High-Level-Rest-Client tests allow deprecation warning temporarily (#2702)" #2744

Merged

5 tasks

tlfeng mentioned this pull request Apr 5, 2022

Test failure v2.0.0#mixedClusterTest o.o.backwards.ExceptionIT caused by 'GET _cat/master' deprecation warning #2759

Closed

tlfeng deleted the mute-hlrc-test-timeout branch April 5, 2022 06:20

tlfeng mentioned this pull request Apr 15, 2022

[2.x] Revert "Make High-Level-Rest-Client tests allow deprecation warning temporarily (#2702)" #2930

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make High-Level-Rest-Client tests allow deprecation warning temporarily, during deprecation of request parameter 'master_timeout'#2702

Make High-Level-Rest-Client tests allow deprecation warning temporarily, during deprecation of request parameter 'master_timeout'#2702
nknize merged 2 commits intoopensearch-project:mainfrom
tlfeng:mute-hlrc-test-timeout

tlfeng commented Apr 1, 2022 •

edited

Loading

Uh oh!

opensearch-ci-bot commented Apr 1, 2022

Uh oh!

dreamer-89 Apr 1, 2022

Uh oh!

tlfeng Apr 1, 2022

Uh oh!

dreamer-89 Apr 1, 2022

Uh oh!

tlfeng Apr 1, 2022

Uh oh!

tlfeng Apr 2, 2022

Uh oh!

opensearch-ci-bot commented Apr 2, 2022

Uh oh!

nknize left a comment

Uh oh!

tlfeng commented Apr 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

tlfeng commented Apr 1, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues Resolved

Check List

Uh oh!

opensearch-ci-bot commented Apr 1, 2022

Uh oh!

dreamer-89 Apr 1, 2022

Choose a reason for hiding this comment

Uh oh!

tlfeng Apr 1, 2022

Choose a reason for hiding this comment

Uh oh!

dreamer-89 Apr 1, 2022

Choose a reason for hiding this comment

Uh oh!

tlfeng Apr 1, 2022

Choose a reason for hiding this comment

Uh oh!

tlfeng Apr 2, 2022

Choose a reason for hiding this comment

Uh oh!

opensearch-ci-bot commented Apr 2, 2022

Uh oh!

nknize left a comment

Choose a reason for hiding this comment

Uh oh!

tlfeng commented Apr 4, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tlfeng commented Apr 1, 2022 •

edited

Loading